Align interleaved source locators with Parquet FILE - #2284
Conversation
3276dbc to
7ec6416
Compare
Greptile SummaryThe PR replaces JSON-string interleaved source locators with a Parquet FILE-compatible struct while moving archive, frame, and PDF provenance into adjacent typed columns. It also consolidates legacy migration at schema alignment and streamlines materialization and Parquet I/O.
Confidence Score: 5/5The PR appears safe to merge because the previously reported compatibility and provenance failures are resolved at the current head. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Legacy JSON source_ref] --> B[align_interleaved_table]
C[Typed WebDataset locator] --> B
B --> D[FILE-compatible source_ref]
B --> E[source_member]
B --> F[source_frame_index]
B --> G[page_number and source_bbox]
D --> H[Materialization]
E --> H
F --> H
D --> I[Parquet writer via fsspec]
E --> I
F --> I
G --> I
Reviews (9): Last reviewed commit: "refactor(schema): share Parquet FILE typ..." | Re-trigger Greptile |
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
528e8a5 to
a58f01d
Compare
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
d86a4a6 to
8fad1aa
Compare
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
|
/okay to test 7a5a8e9 |
|
/claude review |
There was a problem hiding this comment.
Pull request overview
This PR migrates interleaved provenance from a JSON-string source_ref locator to a Parquet FILE-shaped source_ref struct (with adjacent source_member / source_frame_index), and updates schema alignment, materialization, and I/O stages to improve correctness and throughput while preserving backward compatibility via a one-time migration at the schema boundary.
Changes:
- Introduces a canonical Arrow
FILE_REFERENCE_TYPEand updatesINTERLEAVED_SCHEMAto storesource_refas a FILE-compatible struct plus adjacent metadata columns. - Adds
align_interleaved_table()migration/casting logic and updates materialization + readers/writers to operate on typed Arrow tables and fsspec-backed output. - Updates tests and docs to reflect the new locator shape and semantics (including PDF provenance migration to adjacent columns).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/stages/interleaved/test_schema_utils.py | Adds coverage for legacy JSON source_ref migration into typed FILE struct + adjacent columns. |
| tests/stages/interleaved/test_multimodal_writer.py | Updates writer tests for typed source_ref, adjacent member/frame fields, and fsspec-backed Parquet output. |
| tests/stages/interleaved/test_multimodal_reader.py | Updates reader assertions to validate source_member / source_frame_index instead of parsing JSON. |
| tests/stages/interleaved/test_multimodal_core.py | Adjusts core materialization/classification tests to the FILE + adjacent metadata model and unified range-read path. |
| tests/stages/interleaved/test_materialization.py | Extends edge-case coverage (e.g., size-only reads; empty-range acceptance) under new range-read semantics. |
| tests/stages/interleaved/test_interleaved_task.py | Verifies canonical FILE child ordering and validates range constraints in build_source_ref. |
| tests/stages/interleaved/test_base_writer.py | Updates base-writer tests to _write_table and Arrow-table alignment outputs. |
| tests/stages/interleaved/pdf/nemotron_parse/test_utils.py | Adds assertions for migrated PDF provenance fields (page_number, source_bbox) with source_ref=None. |
| tests/stages/interleaved/conftest.py | Updates fixtures/helpers to construct typed FILE source_ref and adjacent metadata columns. |
| nemo_curator/tasks/interleaved.py | Defines FILE_REFERENCE_TYPE, updates INTERLEAVED_SCHEMA, and replaces JSON helpers with FILE struct helpers/expansion. |
| nemo_curator/stages/interleaved/utils/schema.py | Adds align_interleaved_table() to migrate/cast legacy locator shapes to canonical FILE struct and append adjacent columns. |
| nemo_curator/stages/interleaved/utils/materialization.py | Consolidates whole-file and ranged reads into deduped fs.cat_ranges() flow; updates classification inputs/columns. |
| nemo_curator/stages/interleaved/README.md | Documents new FILE-shaped source_ref and adjacent metadata strategy; updates materialization strategy description. |
| nemo_curator/stages/interleaved/pdf/nemotron_parse/utils.py | Migrates Nemotron provenance from embedded JSON source_ref to adjacent page_number / source_bbox. |
| nemo_curator/stages/interleaved/io/writers/webdataset.py | Moves writer implementation to _write_table and converts Arrow → pandas only at the boundary. |
| nemo_curator/stages/interleaved/io/writers/tabular.py | Switches Parquet writing to pq.write_table through the configured fsspec filesystem and sets dictionary defaults. |
| nemo_curator/stages/interleaved/io/writers/base.py | Refactors base writer to prepare/align Arrow tables directly and fast-path Arrow inputs when materialization is unnecessary. |
| nemo_curator/stages/interleaved/io/readers/webdataset.py | Emits typed FILE-compatible source_ref scalars and populates adjacent source_member / source_frame_index. |
| refs = [json.loads(value) if value else {} for value in table.column(source_ref_index).to_pylist()] | ||
| for ref in refs: | ||
| if (path := ref.get("path")) is not None: | ||
| ref.update( | ||
| uri=str(path), | ||
| offset=int(ref["byte_offset"]) if ref.get("byte_offset") is not None else None, | ||
| size=int(ref["byte_size"]) if ref.get("byte_size") is not None else None, | ||
| ) | ||
| table = table.set_column( | ||
| source_ref_index, | ||
| "source_ref", | ||
| pa.array((ref if ref.get("path") is not None else None for ref in refs), type=FILE_REFERENCE_TYPE), | ||
| ) |
| if ( | ||
| ctx.member_info | ||
| and content_key in ctx.member_info | ||
| and not ctx.tar_path.lower().split("?", 1)[0].endswith((".tar.gz", ".tgz")) |
There was a problem hiding this comment.
This .tar.gz/.tgz guard is a subtle correctness fix — info.offset_data/info.size are positions in the decompressed stream, so emitting them as a FILE byte range would make materialization cat_ranges() read garbage from the compressed file. Good catch, but it's new behavior with no direct test. Consider adding a case that reads image members from a gzipped tar and asserts source_ref carries no offset/size (falls back to tar-extract), so a future refactor doesn't silently reintroduce the range read.
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
Summary
Align interleaved source locators with the Parquet
FILElogical-type shape, simplify materialization, and improve actual Parquet reader/writer throughput.source_ref.pathsource_ref.urisource_ref.byte_offsetsource_ref.offsetsource_ref.byte_sizesource_ref.sizesource_ref.membersource_membercolumnsource_ref.frame_indexsource_frame_indexcolumnsource_refhas all six optional FILE children from the specification:uri,offset,size,content_type,checksum, andinline.What changed
FILE_REFERENCE_TYPElives in sharednemo_curator.utils.storage_utilsfor reuse by text, image, video, and interleaved stages.{page, bbox}provenance migrates to typed adjacentpage_numberandsource_bboxcolumns instead of being discarded.uri + sizenow correctly resolves[0, size)whenoffsetis absent.fs.cat_ranges()path, with one call per filesystem backend.storage_options.use_dictionary.Archive-member and TIFF-frame metadata remain adjacent because FILE is a closed group.
source_memberis only needed for tar extraction when a usable FILE byte range is unavailable; ranged and whole-object FILE reads do not depend on it.Compatibility and scope
The change keeps
binary_contentunchanged. TheinlineFILE child is present in the canonical schema, but moving existing payload storage or materialization toinlineis outside this PR.PyArrow 19 can write the exact physical group but cannot emit the new
LogicalType.FILEfooter annotation. The FILE specification merged on 2026-07-26, and the cited parquet-java and arrow-rs implementations are still open/unreleased. This PR therefore avoids a custom Thrift footer rewriter and can adopt native Arrow support when it lands.Performance validation
Compared latest upstream
main(5732455) with this branch on the same CPU host. These are realInterleavedParquetWriterStageandInterleavedParquetReaderStageruns over 200,000 typed Arrow rows; values are medians of seven warm runs.Remote HTTP materialization used a pinned, range-capable 466 KB object and verified every returned payload:
Validation
nemo_curatorand interleaved tests.main, upstreammain, and the PR base all resolve to5732455; the branch is based directly on current main.Diff